-
Notifications
You must be signed in to change notification settings - Fork 13.8k
some more proc_macro
cleanups
#147386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
some more proc_macro
cleanups
#147386
Conversation
rustbot has assigned @petrochenkov. Use |
The second commit makes sense to me, the arena's use is limited, and if the |
self.grow(bytes); | ||
if let Some(a) = self.alloc_raw_without_grow(bytes) { | ||
// SAFETY: the lifetime is extended here, but then constrained again in `alloc_str`. | ||
return unsafe { &mut *(a as *mut _) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to remove the internal mutability without this change?
I'm not sure how it's related to the rest of the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unsafe
is unfortunately needed to avoid an error[E0499]: cannot borrow *self as mutable more than once at a time
. (This would be fixed with -Zpolonius
.)
I removed the loop
because grow
will always produce enough space to hold the data, so it seemed a bit clearer to me this way (with the old code, I thought at first that grow
might need to be called many times).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep the cells then.
The loop can still be refactored to if let
or unwrap_or_else
though.
Several smaller cleanups to
proc_macro
. Commits 1 and 3 seem pretty trivial to me, commit 2 might be worth it or not.Followup to #147166.